fix(metadata-protocol): batchData's by-id upsert chooses its arm by what the caller can read - #28721

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 9 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/concepts/metadata-lifecycle.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))
• content/docs/permissions/system-context.mdx (via canReadObject (literal, a string literal in callerMayReadObject))

⛔ 4 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v16.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))
• content/docs/releases/v17/17-0.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))
• content/docs/releases/v17/17-5.mdx (via canReadObject (literal, a string literal in callerMayReadObject))
• content/docs/releases/v17/17-6.mdx (via canReadObject (literal, a string literal in callerMayReadObject))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6f9ad22003ac11f3c7cc4819824d28f5b59410d4 && git checkout 6f9ad22003ac11f3c7cc4819824d28f5b59410d4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e735b79ea52bfe41a04afd62e5490c60fbad757a 9fb84ac128d303c0e23da06e8376802ff02520cc && git checkout -B drift-repro e735b79ea52bfe41a04afd62e5490c60fbad757a && git merge --no-ff 9fb84ac128d303c0e23da06e8376802ff02520cc

node scripts/docs-audit/affected-docs.mjs --json e735b79ea52bfe41a04afd62e5490c60fbad757a

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs e735b79ea52bfe41a04afd62e5490c60fbad757a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 9fb84ac128d303c0e23da06e8376802ff02520cc
Local-runs: none
Scope: no reviewFace hits — the seven paths decide no face (no published schema, no governed path; the changeset reads Clause-②: no, not the narrowing arm), so this is the at-tier review ruling 5985885287 asks for before landing on the write-door existence boundary, served at the dispatched tier + packages/metadata-protocol/src/protocol-batch-data.ts (callerMayReadObject, runBatchDataLoop cases create / update / upsert), packages/metadata-protocol/src/protocol.ts (runBatchDataLoop host, probeRecord, assertRecordExists, byIdUpdateRecord, searchAll's canReadObject note at :14806-14831), packages/spec/src/contracts/security-service.ts (canReadObject :860-883), packages/spec/src/security/permission.zod.ts (:263-268), packages/plugins/plugin-security/src/security-plugin-object-admission.ts (canReadObject, objectReadAdmission), packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts (step 2.7, :905-1157), packages/plugins/plugin-security/src/security-plugin.ts (callerCannotReadAddressedRow, absentUnderCallerRead, readUnlessRefused), packages/plugins/plugin-security/src/security-plugin-rls-filter.ts (:312-325), packages/objectql/src/engine.ts (createWithAutonumberResync :8419-8500, the three driver.create sites), packages/drivers/driver-memory/src/memory-driver.ts (create :836, markDirty :2661), docs/adr/0120 (2026-11-01 amendment on origin/main), .changeset/batch-upsert-by-id-caller-read.md, the four test files of the diff.

Read-only shape held: the diff at this head (git diff origin/main...FETCH_HEAD), rulings 5985885287 (#21771) and 5934879010 (#21185), ADR-0120's amendment as merged by #28694, the sibling record 6561342764 on #28731, the head's check runs (68 runs: 57 success, 11 skipped, 0 failure), the PR's one comment (docs-drift). Nothing built or run. PR author is objectstack-fleet[bot]; Implemented-by is the branch.

① Derived judgments

The one changed body is the case 'upsert' fork of runBatchDataLoop for a row carrying an id (protocol-batch-data.ts :370-440). Before: probeRecord (system context) decided update-vs-insert on existence. Now: callerMayReadObject (object-level, once per loop, the security service's canReadObject with the loop's context) and then the caller's own engine.findOne({ where: { id }, context }); a truthy row takes the unchanged update arm, anything else takes the SAME engine.insert call and row builder as case 'create'. No fallback insert on either arm (kept). Judged per principal class, the question being whether any row can LAND at this head that could not before:

1. read+edit, row visible to the caller — canReadObject true, findOne returns the row, update arm, engine.update judges the write as before. Unchanged (dogfood showcase_private_note own row, walled B editor on B's row). Correct.
2. read-only (read, no edit), row visible — update arm, engine.update's CRUD gate refuses 403 PERMISSION_DENIED, nothing lands. Unchanged (dogfood showcase_task, walled B creator on B's row; showcase_account control: one refusal for a held and an unknown id). Correct.
3. read grant held, row hidden by RLS / sharing / the organization wall — findOne returns null (hidden rows answer absence, not a throw: this is the same value == null the security layer's own absentUnderCallerRead reads as "cannot read"), insert arm, engine.insert collides on the primary key → 409 UNIQUE_VIOLATION, the row's errors deep-equal to a batch create's for the same id (unit case 1; dogfood showcase_inquiry, showcase_private_note hidden, both walled personas on A's row, names neither A's id, name nor slug). Before: update arm → the pre-image check's 404 (or 403) beside a 201 for an unknown id, i.e. the existence signal ruling 5985885287 closed on the other by-id doors. Correct, and the intended change. Nothing lands on a store that enforces the key.
4. edit WITHOUT read (permission.zod.ts :263-267 derives allowRead and allowEdit independently; only modifyAllRecords implies read) — canReadObject asks the find grant (objectReadAdmission step 5, checkObjectPermission('find', …)), so it answers false and EVERY row of the loop takes the insert arm without a row read (unit case 3). The insert arm never reads the edit bit: engine.insert judges allowCreate. So: without create → the create's 403; with create (the create+edit-without-read class) → 409 UNIQUE_VIOLATION on a PK-enforcing store. Nothing lands at this head. ⚠️ Before this diff, on this door, the class COULD land: the old fork sent the existing row to the update arm, and the security layer's step 2.7 (security-plugin-authz-middleware.ts :1146-1156) answers a by-id write whose read is REFUSED (a declared 4xx — the object's read grant withheld) by keeping today's admission when no write-class row filter binds (computeRlsFilter null: security-plugin-rls-filter.ts :312-325 derives the write class from the select class and leaves it empty when both are empty and the tenant wall does not apply) — the class record 6561342764 named on fix(metadata-protocol): the by-id OCC comparison answers only for a row the caller reads #28731 at the same seam. This diff CLOSES that path on the batch upsert door, because the update arm is now unreachable without a successful caller read. A removed landing, not an added one. Consequence for the PR text, under ③ and ⑤ below.
5. create-only, no read — canReadObject false → insert arm → 409 for a held id, 201 for an unknown one, as the plain create answers (dogfood creator persona). Before: update arm → 403. Refusal to refusal. Correct.
6. admins (modifyAllRecords / viewAllRecords), system context (isSystem) — admission true (system bypass, or read implied by the bypass bit), findOne sees the row, update arm. Unchanged. Correct.
7. context-less in-process call (context === undefined) — not asked; findOne without a context, as its writes run. Unchanged (unit case 6). A null context is asked and the plugin answers false (principal-less, ADR-0096 D5) → insert arm, fail-closed. Correct.
8. cross-organization, walled posture — the wall hides A's row from B's reader, insert arm, the insert carries B's organization and collides on the deployment-global id → 409, exactly ruling 5934879010 rule 2 extended by the 2026-11-01 amendment; the fresh id lands in B. Correct (dogfood walled suite, atomic and not).
9. objects with write-class RLS — a readable row the write policy refuses: update arm, step 2.7 refuses 403 as before. Unchanged.
10. driver paths — engine.insert reaches the store only through driver.create (engine.ts :8443, :8499, :16017); driver.upsert is never on this path, so no ON CONFLICT DO UPDATE can turn the insert arm into an overwrite. createWithAutonumberResync retries a unique violation only for a number the engine or driver issued; a caller-supplied id collision is enveloped on the first attempt (an object with an autonumber field whose driver does not name the colliding column re-issues the number and fails again on the same id — ERR_AUTONUMBER_COLLISION, nothing written; identical to case 'create', pre-existing). driver-memory's create pushes a second row with the same id (assertUnique enforces field-level unique only) — a DUPLICATE row for the classes in 3/4/5 above where the old fork answered a refusal. ADR-0120's amendment records exactly this for every create door ("not a production store, declares no primary key, so a duplicate id lands there"); flagged under ③ because markDirty can persist it through a persistenceAdapter.
11. atomic arm — the loop runs inside runAtomicBatch with the transaction's context, so both the admission and the row read ride the transaction (unit case 10: both findOne calls carry the handle), the causal row carries UNIQUE_VIOLATION, earlier rows ROLLED_BACK. Correct. Non-atomic with continueOnError: per-row, as before (unit case 11).
12. callerMayReadObject semantics — (a) a throwing canReadObject fails the row (and the batch, atomic): stricter than ISecurityService.canReadObject's "callers must treat a throw as false" (security-service.ts :863-865) but fail-closed, the same stance searchAll took for the same method (protocol.ts :14826-14827); the shipped plugin never throws (it catches and answers false), and a throw gives the same answer for a held and an unknown id, so nothing is disclosed. Acceptable; noted under ③. (b) absent security service or a service predating the method → true, "not refused at this layer": the row read is still the middleware's, which refuses the object-level non-reader with 403 for every id — stricter than the create's answer, and the same for held and unknown ids. The contract text (:875-881) says compose the verdict from explain instead; searchAll already reasons as this fork does. No hole; noted under ③. (c) The once-per-loop cache is safe: one loop = one object and one context (a batch names one object; the atomic loop is one invocation under one transaction context), and a rejected promise is awaited on the same row that created it and re-awaited by later rows, so no unhandled rejection and every row answers alike.
13. public face — BatchDataHost (exported interface of a non-entry module) trades probeRecord for getServicesRegistry; probeRecord's other readers (OCC) untouched; protocol.ts's host object matches. No packages/spec change; no new error code (UNIQUE_VIOLATION is the registered one). The findOne options are typed EngineQueryOptionsParsed (no as any), and check:query-options-erasure reports none new.

Net: at this head no principal class can land a row it could not land before; one class (4) loses a landing the governing text never granted. Timing (a reader's extra findOne before the insert) is outside the ruling's shape and unchanged in kind from the old probe.

② Semver level

@objectstack/metadata-protocol patch — right. Clause-②: no is right: the accept set is not widened and no public face grows; the one movement is the removed landing in ① 4, which is a runtime security-behaviour change (execution-duties 负边界: not Clause-②) and, read as an accept-set narrowing, is still spelled no (execution-duties: 收窄已发布接受集的卡是 Clause-②: no, owing exactly this at-tier review). The (narrowing) arm is BREAKING and is for a declared capability taken away; ruling 5985885287 already governs that a row the caller cannot read does not exist on the write doors, so nothing declared is removed. Metadata-protocol: none — right, no authorable key moves. Skills-docs: none with reason — consistent with git grep of skills/ as the body states. Changeset body matches the diff: readable row updated under the caller's write permissions, unreadable id takes the create arm and answers as a create naming it, id-less rows create, atomic rollback, system and context-less calls unchanged. One sentence to correct in the PR body (not the changeset): "A row it cannot read was never written by this door" is true of the ownership-floor domain and of every reader, and false for class ① 4 outside a write-class row filter — the body should say the arm is now unreachable for that class rather than that it never landed.

③ Boundary flags

• Author objectstack-fleet[bot]; Implemented-by is the dev branch. Dev flags, each answered:
• driver-memory duplicate (body "Acceptance notes", fork comment) — acknowledged; the amendment rules it an accepted residual of every create door and this door now IS a create door for an unreadable id. Residual: with a persistenceAdapter (markDirty) the duplicate is persisted in a dev store. Not a must-fix (ruled); the docs-only follow-up below should say so.
• ADR-0120 amendment's door list — the amendment names the create doors (POST /data/:object, batch create, createMany, cross-object /batch, import insert/upsert, clone) and binds "⛔ No create door answers a collision on id differently by the visibility of the row that holds it". The batch upsert by-id fork is now such a door and is not listed. The seat's planned docs-only PR should add it (and note that this door, unlike import upsert, answers a held-but-unreadable id with the collision rather than a match). Should-fix, separate PR as planned.
• Pin coverage, class ① 4 (edit without read, with and without create) — no pin at this head: the dogfood personas hold read on every object they aim at except the create-only creator; the unit battery's object-refused case uses a context with no grants at all. The class the hardening closes is therefore unmeasured, and the ablation could not have gone red on it. Not a must-fix for landing safety: the update arm is structurally unreachable without a successful caller read (object admission asks the find grant; the insert arm never reads the edit bit), and the create-only persona measures the same code path. Should-fix: add an allowRead: false, allowEdit: true principal (±allowCreate) to the dogfood HIDDEN table on an object outside the ownership floor's domain, asserting the create's answer and the stored row untouched, so the one class this PR exists for is pinned by name.
• Contract text vs. this fork (① 12 a/b): ISecurityService.canReadObject's docblock asks callers to treat a throw as false and to compose from explain when the method is absent; this fork (and searchAll before it) fails the row on a throw and lets the middleware's own refusal stand when absent. Both stricter, both uniform across held/unknown ids. Should-fix, docs: either the fork's docblock cites searchAll as the precedent it follows (it does) and the contract's note is widened to admit "fail the read" as a fail-closed reading, or nothing; no code change needed here.
• Docs drift (comment 6559948159): content/docs/permissions/system-context.mdx names canReadObject's callers (the elevation reads); the new caller is classified not-a-door in the core pin (row-serving-door-exposure.pin.test.ts), which is right — it chooses an arm, serves no row, and its true still reads through the middleware. The page is prose face: the seat's ACCEPT names whether a sentence is owed.
• Fork comment accuracy: the ⚠️ paragraph's first reason names the 403/404-beside-201 signal; it does not name the third outcome of the old fork (the landing in ① 4). Should-fix, one clause.
• Clause-②: no declared without the (narrowing) arm — judged right in ②; no open_questions block in the body; the body's "Changed" and "Unchanged" bullets hold against the diff.

④ Design judgment

No face asks it; recorded as the ruling-asked judgment of the fork against the text it implements. Ruling 5985885287 A: "a row the caller cannot read answers exactly what a nonexistent id answers" — on the upsert door a nonexistent id answers a create, and a create naming a held id has only the collision to give; the 2026-11-01 amendment states that reconciliation ("a create naming a held id has no 'nonexistent' answer to give, because the key it names is taken") and binds the answer to be the create's, with no detail of its own. The fork meets it: same engine.insert, same row builder, no new code, field or sentence; hidden-in-own-organization and other-organization rows answer alike (dogfood both suites). Ruling 5934879010 rule 2 is followed for the collision's code. The reversal of #5099/#5187 is recorded in the fork comment with the right citations and keeps #5099's no-fallback half. Holds.

Implemented-by: claude/harden-upsert-by-id-visibility
Reviewed-by: session_017xRBcVy4VXmkQjn2MGeaWk

VERDICT: PASS

Must-fixes: none. Should-fixes: (1) PR body "Acceptance notes" — replace "was never written by this door" with the ① 4 reading (the arm is now unreachable for the edit-without-read class; before this diff that class could land where no write-class row filter bound); (2) pin the ① 4 class in the dogfood HIDDEN table (±create); (3) the planned docs-only PR adds the batch upsert by-id door to ADR-0120's amendment list, with the persisted driver-memory residual; (4) one clause in the fork comment naming the old fork's third outcome.

Reading time 2026-11-01T17:58Z.

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

ACCEPT → head 9fb84ac128d303c0e23da06e8376802ff02520cc · 2026-11-01T18:02Z

维护者速读

• 改了什么：批量 upsert 按 id 选更新还是插入，从"系统身份查记录在不在"改为"调用方自己读不读得到"。读得到就走更新，仍由引擎判写权限；读不到就走和 case 'create' 完全相同的插入，撞键时回答与创建相同的 409 UNIQUE_VIOLATION,不点名组织。
• 为什么改：QA 第三轮发现的安全加固(细节按披露裁决不公开)。
• 风险与回滚：同一行对调用方读不到时，回答由原来的 403/404 改为创建的 409。读得到的行照常更新，不存在的 id 照常创建，系统上下文不变。回滚即 revert 本 PR。
• 先看：packages/metadata-protocol/src/protocol-batch-data.ts(callerMayReadObject 与 upsert 分叉)。
• 两分钟验证：pnpm --filter @objectstack/dogfood exec vitest run test/upsert-by-id-caller-read.dogfood.test.ts,预期 28/28 通过。
• 需要你做的：无，席位落地。

席位意见：收。本 PR 落实维护者 2026-11-01 对 ② 的裁定(a,原话「同意」),与 #28694 合入的 ADR-0120 修订注记一致。达档复核 6561724481 为 PASS,无 must-fix。

席位核对

1. 形态：draft,目标 main;按披露裁决不建卡，正文没有关闭关键词。正文由席位写入最终读数；复核 should-fix 1 指出的那句不准确，也已改成中性表述。
2. 前提实测：ablation 把分叉回退到系统身份探测，单元测试 8 个变红、dogfood 10 个变红，恰好是"隐藏行 / 跨组织"两类用例；恢复后 blob 等于 HEAD,preflight --absent exit 0。
3. 门禁：本地派生 67 项，全部跑完。完整 dogfood 跑了 372 个文件、0 失败。本 head 的 CI 共 72 项：59 success、13 skipped、0 失败；Lint & Repo Gates、TypeScript Type Check、Test Core、Check Changeset 都是 success。
4. 契约面：没有碰契约四面，Clause-②: no。按裁决 5985885287 的要求，在"写入口是否暴露记录存在"这条边界上做了达档复核，结果 PASS。复核逐类核对了各种调用方，没有任何一类能写进以前写不进的行。
5. 范围外去向：复核 should-fix 2–4 并入排队中的后续 PR(只改文档和测试)，内容是：
   • 把 batch upsert 入口补进 ADR-0120 修订注记的入口列表，并写明 driver-memory 的残留；
   • 给"有写权无读权"这一类调用方补一个 pin;
   • 在分叉注释里补一句说明。
6. 落地：不属于受管面，777 行，低于 3000 行上限 → pr_ready + automerge_enable。

Generated by Claude Code